Skip to content

Conversation

@6eanut
Copy link
Contributor

@6eanut 6eanut commented Jan 22, 2026

This patch enables syzos for riscv64 and implements the corresponding pseudo syscalls.

Pseudo syscalls:

  • syz_kvm_setup_syzos_vm
  • syz_kvm_add_vcpu
  • syz_kvm_assert_syzos_uexit

Syzos guest support:

  • guest_uexit
  • guest_execute_code
  • guest_handle_csrr and guest_handle_csrw

Test seeds:

  • riscv64-syz_kvm_setup_syzos_vm
  • riscv64-syz_kvm_setup_syzos_vm-csrr
  • riscv64-syz_kvm_setup_syzos_vm-csrw

The partial printout information of syz-execprog -debug -threaded=0 riscv64-syz_kvm_setup_syzos_vm/riscv64-syz_kvm_setup_syzos_vm-csrr/riscv64-syz_kvm_setup_syzos_vm-csrw is as follows. It can be seen that the executions were successful.

  • riscv64-syz_kvm_setup_syzos_vm
image
  • riscv64-syz_kvm_setup_syzos_vm-csrr
image
  • riscv64-syz_kvm_setup_syzos_vm-csrw
image

Before sending a pull request, please review Contribution Guidelines:
https://github.com/google/syzkaller/blob/master/docs/contributing.md


@6eanut 6eanut force-pushed the upstream/syzos branch 2 times, most recently from 1052267 to 4fe8ded Compare January 22, 2026 06:29
return;
if (cmd->size > size)
return;
switch (cmd->call) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried building with Clang? It is actually pretty aggressively turning switches into jump tables.
Perhaps it will start biting soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I mainly based the adaptation on arm64. I’ll modify it to use if-else statements instead.

#define AUIPC_OPCODE 0x17
#define AUIPC_OPCODE_MASK 0x7f

// Code loading SyzOS into guest memory does not handle data relocations (see
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change to "SYZOS".
The naming is still inconsistent across the codebase, but I've decided "SYZOS" is better than "SyzOS", and now the former prevails.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged.

// Setup executor guest code.
struct addr_size host_text = alloc_guest_mem(&allocator, 4 * KVM_PAGE_SIZE);
install_syzos_code(host_text.addr, host_text.size);
vm_set_user_memory_region(vmfd, slot++, KVM_MEM_READONLY, SYZOS_ADDR_EXECUTOR_CODE, host_text.size, (uintptr_t)host_text.addr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take a look at how setup_vm() is implemented for amd64.
While this code (apparently based on the executor/common_kvm_arm64.h) is fine, the x86 implementation does a better job factoring out the memory layout configuration.

Copy link
Contributor Author

@6eanut 6eanut Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right. I have modified the riscv64 implementation to refer to the x86 implementation.

@@ -0,0 +1,19 @@
#
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried generating C reproducers out of these programs and ensuring that the assertions pass?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I have verified this. I first generated a C reproducer using
./bin/syz-prog2c -os=linux -arch=riscv64 -repeat=1 -prog=sys/linux/test/riscv64-syz_kvm_setup_syzos_vm-csrw > repro.c.
The generated repro had a small type typo (uint64_t_t), which I manually fixed to uint64_t to make it compile.
I then cross-compiled the program for riscv64, ran the resulting binary inside a QEMU riscv64 VM, and the syzos assertions passed as expected.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's because executor code is using uint32, uint64 etc. instead of uint32_t, uint64_t. Just make sure you don't have uint64_t in the SYZOS headers.

uint64 size;
};

struct api_call_uexit {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using api_call_1 instead.
It was silly of me to try and invent a separate struct for each API call.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'll change it.

uint32 insns[];
};

struct api_call_1 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While at it, can you please move api_call_header, api_call_1, api_call_2 etc. to executor/common_kvm_syzos.h? I think it's long overdue.

You can probably keep api_call_code here for now, as it is arch-specific.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'll do it.

if (size > guest_mem_size)
size = guest_mem_size;
memcpy(host_mem, text, size);
memcpy(host_mem + page_size, (void*)guest_unexp_trap, KVM_PAGE_SIZE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guest_unexp_trap() is a guest function, how about moving it into the SYZOS header?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it’s better to keep guest_unexp_trap() here.
SYZOS guest functions are meant to be mutable via syz_kvm_add_vcpu parameters, while guest_unexp_trap() is part of the fixed vCPU setup path and not intended to be mutated.

What do you think ?~

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SYZOS guest functions are meant to be mutable via syz_kvm_add_vcpu parameters

I don't understand. All of SYZOS code is immutable, we can only pass parameters to it.

What is the purpose of guest_unexp_trap(), is it something like a default interrupt handler?
In that case, it is probably meant to live in SYZOS.

Also, right now these functions are not marked as GUEST_CODE. If e.g. guest_unexp_trap() happens to call sbi_ecall() instead of inlining it, everything will break, because sbi_ecall() won't be in the guest address space.

@6eanut
Copy link
Contributor Author

6eanut commented Jan 22, 2026

@ramosian-glider

Thanks for the review and suggestions. I’ve made the changes and updated the commit. Please take a look.

@6eanut 6eanut force-pushed the upstream/syzos branch 3 times, most recently from 7317dc0 to 42cebab Compare January 23, 2026 03:34
This patch enables syzos for riscv64 and implements
the corresponding pseudo syscalls.

Pseudo syscalls:
  - syz_kvm_setup_syzos_vm
  - syz_kvm_add_vcpu
  - syz_kvm_assert_syzos_uexit

Syzos guest support:
  - guest_uexit
  - guest_execute_code
  - guest_handle_csrr and guest_handle_csrw

Test seeds:
  - riscv64-syz_kvm_setup_syzos_vm
  - riscv64-syz_kvm_setup_syzos_vm-csrr
  - riscv64-syz_kvm_setup_syzos_vm-csrw
size_t size = (char*)&__stop_guest - (char*)&__start_guest;
if (size > mem_size)
fail("SyzOS size exceeds guest memory");
fail("SYZOS size exceeds guest memory");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the naming in the existing code, but please put it into a separate commit.

SYZOS_API_STOP, // Must be the last one
} syzos_api_id;

struct api_call_header {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this a separate commit that only touches x86/ARM and brings no functional change.


// Code loading SyzOS into guest memory does not handle data relocations (see
// https://github.com/google/syzkaller/issues/5565), so SyzOS will crash soon after encountering an
// Code loading SYZOS into guest memory does not handle data relocations (see
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here - please group all s/SyzOS/SYZOS into a separate commit.

return -1;
}

uint64_t actual_code = ((uint64_t*)(run->mmio.data))[0];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where uint64_t should be replaced with uint64.

r0 = openat$kvm(0, &AUTO='/dev/kvm\x00', 0x0, 0x0)
r1 = ioctl$KVM_CREATE_VM(r0, AUTO, 0x0)
r2 = syz_kvm_setup_syzos_vm$riscv64(r1, &(0x7f0000c00000/0x400000)=nil)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this test, and the following one, can you please add a line or two describing what they are supposed to test?

}

# Table 5 in https://docs.riscv.org/reference/isa/_attachments/riscv-privileged.pdf .
riscv64_csr = 0x100, 0x104, 0x105, 0x140, 0x141, 0x142, 0x143, 0x144, 0x180, 0x106, 0x10a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 18 registers in Table 5, why aren't you including all of them?
(Maybe what you are doing is right, but please explain that in the comment)

if (size > guest_mem_size)
size = guest_mem_size;
memcpy(host_mem, text, size);
memcpy(host_mem + page_size, (void*)guest_unexp_trap, KVM_PAGE_SIZE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SYZOS guest functions are meant to be mutable via syz_kvm_add_vcpu parameters

I don't understand. All of SYZOS code is immutable, we can only pass parameters to it.

What is the purpose of guest_unexp_trap(), is it something like a default interrupt handler?
In that case, it is probably meant to live in SYZOS.

Also, right now these functions are not marked as GUEST_CODE. If e.g. guest_unexp_trap() happens to call sbi_ecall() instead of inlining it, everything will break, because sbi_ecall() won't be in the guest address space.

// implementation in Linux kselftest KVM RISC-V tests.
// See https://elixir.bootlin.com/linux/v6.19-rc5/source/tools/testing/selftests/kvm/lib/riscv/processor.c#L337 .

#define KVM_RISCV_SELFTESTS_SBI_EXT 0x08FFFFFF
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove "SELFTESTS" from the name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants